fix(checker): general non-overflowing AST teardown + measured MAX_EXPR_DEPTH (#37) by hyperpolymath · Pull Request #43 · hyperpolymath/my-lang

hyperpolymath · 2026-05-17T02:13:52Z

Closes the Linux-side of #37 (subtleties 1 & 3). Windows-CI stack measurement still open (needs CI access).

1. General iterative AST teardown (subtlety 1)

The recursive-Drop overflow is shape-independent; the old test-only drop_program_iteratively only handled 2-arg Call chains. Replaced with ast::drop_program_iteratively: owned nodes popped from an explicit heap worklist, destructured by value (legal because Expr has no Drop impl), every recursive child moved into the worklist before the shallow remainder drops. O(n) time, O(n) heap, O(1) stack, zero shape assumptions; covers every recursive edge (Box<Expr>, Vec<Expr>, Block/Stmt, Match/Record/Lambda/Ai). Measured: tears down a 1,000,000-deep non-Call chain on a 512 KiB stack (recursive Drop cliffs at ~5k).

impl Drop for Expr was deliberately rejected — it forbids by-value destructuring of Expr across the parser/HIR/MIR crates. Documented inline (the issue's "or justify an alternative" clause).

2. `MAX_EXPR_DEPTH` re-derived from a measured budget (subtlety 3)

New examples/measure_depth.rs probes one check_expr walk on a fixed thread stack: ≈4.4 KiB/level. The checker is never put on a large-stack thread, so the binding constraint is the OS main thread = 1 MiB on Windows. Old 256 × 4.4 KiB ≈ 1.14 MiB — it can overflow before the guard fires on Windows (not merely "unjustified" as the issue states, but unsafe there). Re-derived 256 → 128 (~569 KiB, ~54% of 1 MiB; ~46% headroom). Parser's independent MAX_PARSE_EXPR_DEPTH = 64 is the real operational ceiling, so no parseable program reaches the guard — lowering it rejects zero real code. Stale "quarter of 256" parser docs corrected.

3. Regression test (DoD item 4)

test_deep_non_call_ast_teardown_does_not_overflow — a 50,000-deep Unary chain (no Call), the exact shape the old helper could not handle. checker_alloc_scaling depth ladder made constant-relative so it can't silently exceed a re-tuned guard.

Verification

cargo test -p my-lang fully green: 137 unit + 3 alloc-scaling + 15 + 1, 0 failed. Minimal diff (235+/49−, 4 files + 1 new example) — no crate-wide reformat.

DoD status

Measure stack-frame cost per recursion level (Linux) — ≈4.4 KiB/level checker; ≈133 B/level recursive Drop
Replace shape-specific drop helper with a general non-overflowing teardown
Re-derive MAX_EXPR_DEPTH from the measured budget (256 → 128) + update docs(checker): reframe MAX_EXPR_DEPTH as a stack-recursion bound (closes #14) #36-reframed docs
Regression test with a deep, non-Call-shaped AST
Windows-CI leg of the measurement (needs CI access; harness shipped so it's now one command there)

🤖 Generated with Claude Code

…R_DEPTH (#37) Closes the Linux-side of #37 (subtleties 1 & 3). 1. General iterative AST teardown (subtlety 1) The recursive-`Drop` stack overflow is shape-independent; the old test-only `drop_program_iteratively` only flattened 2-arg `Call` chains. Replaced with `ast::drop_program_iteratively`: pops owned nodes from an explicit heap worklist and destructures them by value (legal because `Expr` has no `Drop` impl), moving every recursive child into the worklist before the shallow remainder drops. O(n) time, O(n) heap, O(1) stack, zero shape assumptions. Covers every recursive edge (Box<Expr>, Vec<Expr>, Block/Stmt, Match/Record/ Lambda/Ai). Measured: tears down a 1,000,000-deep non-Call chain on a 512 KiB stack (recursive Drop cliffs at ~5k). `impl Drop for Expr` was deliberately rejected: it would forbid by-value destructuring of `Expr` across the parser/HIR/MIR crates ("cannot move out of a type which implements Drop"). Documented. 2. MAX_EXPR_DEPTH re-derived from a measured budget (subtlety 3) `examples/measure_depth.rs` probes one `check_expr` walk on a fixed-size thread stack: ~4.4 KiB/level. The checker is never dispatched onto a large-stack thread, so the binding constraint is the OS main thread = 1 MiB on Windows. The old 256 needs ~1.14 MiB -- it could overflow *before* the guard fires on Windows (not just "unjustified" as the issue said, but unsafe there). Re-derived to 128 (~569 KiB, ~54% of a 1 MiB Windows stack). The parser's independent MAX_PARSE_EXPR_DEPTH=64 is the real operational ceiling, so no parseable program reaches the guard and lowering it rejects zero real code. Stale "quarter of 256" parser docs corrected. 3. Regression test (DoD item 4) `test_deep_non_call_ast_teardown_does_not_overflow`: a 50,000-deep `Unary` chain (no `Call`) -- the exact shape the old helper could not handle. `checker_alloc_scaling` depth ladder made constant- relative so it can't silently exceed a re-tuned guard. Still open in #37: the Windows-CI half of the stack measurement (needs CI access; the Linux measurement + harness land here). Full `cargo test -p my-lang` green (137 unit + 3 alloc-scaling + rest). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

hyperpolymath · 2026-05-17T04:56:50Z

Drive-by review from a parallel session that independently re-derived this exact fix (same iterative heap-worklist teardown, same by-value destructuring + explicit impl Drop rejection, same 256→128 from a ~5 KiB/level measurement, same parser-doc and scaling-ladder corrections). Independent convergence is good evidence the design is right — no competing PR; one suggestion for the last DoD box instead.

Closing the open "Windows-CI leg" item without needing CI access. The measurement is shipped as examples/measure_depth.rs (a manual one-command run). If the same probe lived in a #[cfg(test)] unit test instead of an example, it would run automatically on the existing windows-latest matrix leg of .github/workflows/checker-scaling.yml — turning the unchecked box into a permanent per-change Windows guard rather than a manual step.

Sketch that worked in the parallel session:

A #[cfg(test)] pub mod stack_probe with an #[inline(never)] probe that records the stack-pointer high/low watermark, plus a one-line #[cfg(test)] crate::stack_probe::note(); at the top of check_expr_guarded (compiles to nothing in release — production hot path untouched; verified with a release build).
The measurement as a normal #[test] running on a generous spawned stack, asserting the per-level cost is stable across two depths (linear-in-stack is the property that justifies a single constant) and under a sanity ceiling. Pin the probe depths to MAX_EXPR_DEPTH / 3 and MAX_EXPR_DEPTH * 9 / 10 so they stay under the guard if the limit is re-derived again (probing at a fixed 240 silently breaks once the limit drops to 128 — the guard truncates the recursion and the per-level figure halves).

Note for integration tests: #[cfg(test)] items are not visible to the tests/ crate (it links the lib built without cfg(test)), so the probe + its test must live in the lib, not in tests/.

Optional: a companion docs/wiki/internals/ decision record (sibling to checker-allocation-investigation.md) capturing the measured numbers and the free-fn-vs-impl Drop rationale, matching the #14 doc's "future work starts from this baseline" practice.

Everything else here LGTM as-is.

github-actions · 2026-05-17T10:13:53Z

🔍 Hypatia Security Scan — findings in this PR's changed files

2 finding(s) in files this PR modifies (of 32 repo-wide; 0 baselined, not shown).

Severity	Count
🔴 Critical	0
🟠 High	1
🟡 Medium	1

View findings (this PR's files only)

[
  {
    "reason": "expect() in hot path (157 occurrences, CWE-754)",
    "type": "expect_in_hot_path",
    "file": "/home/runner/work/my-lang/my-lang/crates/my-lang/src/parser.rs",
    "action": "flag",
    "rule_module": "code_safety",
    "severity": "medium"
  },
  {
    "reason": "unwrap() without prior check -- DoS via panic (2 occurrences, CWE-754)",
    "type": "unwrap_without_check",
    "file": "/home/runner/work/my-lang/my-lang/crates/my-lang/examples/measure_depth.rs",
    "action": "flag",
    "rule_module": "code_safety",
    "severity": "high"
  }
]

Pre-existing repo-wide findings are triaged via the Hypatia backlog issue and suppressed here by .hypatia-baseline.json / .hypatia-ignore.

Powered by Hypatia Neurosymbolic CI/CD Intelligence

) * fix(checker): self-driving stack-depth measurement + CI guard (#37) Closes the one open DoD item on #37: the Windows-CI leg of the stack-budget measurement confirming MAX_EXPR_DEPTH=128 is safe on the 1 MiB msvc main-thread stack. Subtleties 1 & 3 were fixed in #43; the value was left open until that datapoint, and nothing yet ran the probe. - examples/measure_depth.rs: rewrite the single-shot probe into a self-driving measurement. It re-execs itself as worker subprocesses, binary-searches the overflow cliff for the recursive Drop, the guarded checker walk and the iterative teardown, prints per-platform bytes/level, and exits non-zero if MAX_EXPR_DEPTH no longer fits the 1 MiB floor with headroom. One command now produces and asserts the datapoint on any platform (was a probe needing a manual exit-code wrapper). - .github/workflows/stack-depth.yml: run that measurement and the regression test on ubuntu-latest + windows-latest (same pinned actions as checker-scaling.yml), so the Windows datapoint is produced and locked in on every change instead of relying on a manual run. - tests/stack_depth_37.rs: assert the depth-128 guarded walk and a 1e6-deep iterative teardown both survive a 1 MiB stack. - checker.rs: document that the budget is now reconfirmed automatically on both OSes rather than by a one-off manual measurement. https://claude.ai/code/session_013JnrUmkCpMHsABRmhLVyd2 * fix(measure_depth): harden survives() against false PASS (#37 review) Review of the self-driving measurement flagged that survives() collapsed both subprocess spawn-failure and any non-zero exit into false ("cliffed"), with no positive proof the walk ran — an infra failure (or a future path that exits 0 without doing the work) could skew a measured cliff or risk a false PASS. - survives() now distinguishes three outcomes: spawn-failure / exit-0-without- running-the-walk are fatal (exit 3, never a datapoint); a survived run requires exit 0 AND the worker's "OK ..." completion line; a non-zero exit is the genuine overflow cliff signal. - Document that overflow-aborts-the-process is the cliff mechanism and that join().is_err() only covers the unwinding-panic path (default panic=unwind; abort would also exit non-zero, so either strategy is safe). Driver still PASSes locally (MAX_EXPR_DEPTH=128 safe within 819 KiB of the 1 MiB floor); the binary searches were independently verified correct. https://claude.ai/code/session_013JnrUmkCpMHsABRmhLVyd2 --------- Co-authored-by: Claude <noreply@anthropic.com>

hyperpolymath mentioned this pull request May 17, 2026

Recursive AST Drop stack-overflows on very deep inputs; revisit MAX_EXPR_DEPTH value #37

Closed

4 tasks

hyperpolymath merged commit d965d6f into main May 17, 2026
23 checks passed

hyperpolymath deleted the chore/my-lang-37-iterative-ast-teardown branch May 17, 2026 10:48

hyperpolymath mentioned this pull request May 30, 2026

fix(checker): self-driving stack-depth measurement + CI guard (#37) #80

Merged

4 tasks

hyperpolymath mentioned this pull request May 30, 2026

docs(changelog): record #80 stack-depth CI guard (closes #37) #81

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(checker): general non-overflowing AST teardown + measured MAX_EXPR_DEPTH (#37)#43

fix(checker): general non-overflowing AST teardown + measured MAX_EXPR_DEPTH (#37)#43
hyperpolymath merged 1 commit into
mainfrom
chore/my-lang-37-iterative-ast-teardown

hyperpolymath commented May 17, 2026

Uh oh!

hyperpolymath commented May 17, 2026

Uh oh!

github-actions Bot commented May 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hyperpolymath commented May 17, 2026

1. General iterative AST teardown (subtlety 1)

2. MAX_EXPR_DEPTH re-derived from a measured budget (subtlety 3)

3. Regression test (DoD item 4)

Verification

DoD status

Uh oh!

hyperpolymath commented May 17, 2026

Uh oh!

github-actions Bot commented May 17, 2026

🔍 Hypatia Security Scan — findings in this PR's changed files

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

2. `MAX_EXPR_DEPTH` re-derived from a measured budget (subtlety 3)